CSHARP-2450 (second attempt): Performance: Reduced lock contention in BsonSerializer.LookupActualType #433
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
added 3 commits
February 1, 2021 21:05
…KnownTypesAreRegistered (called by BsonSerializer.LookupActualType which is on the hot path during deserialization) by switching to the (unfortunately not generic) Hashtable type instead of a HashSet<T>. Hashtable is safe for concurrent reads (as per https://docs.microsoft.com/en-us/dotnet/api/system.collections.hashtable?redirectedfrom=MSDN&view=netframework-4.7.2#thread-safety). This is a safe change since the "__typesWithRegisteredKnownTypes" variable is really just a local cache to avoid registering already registered types. It has no effect anywhere else.
… without read lock)
…pe which is on the hot path during deserialization by locking a single dictionary lookup only. The HashSet<T> instances that are tracked inside the Hashtable are kind of immutable now as they get wholly replaced inside the dictionary instead of updated which allows consumers to safely iterate over the old instances, thus allowing the read-locked section to be shortened.
dnickless
force-pushed
the
CSHARP-2450
branch
from
984a222 to
62cd31e
Compare
|
Any chance someone could look at this? We're eagerly waiting to see this or the other other PR #347 in a next version of the driver. Thanks! |
|
Hi, @dnickless, We have investigated the performance improvements observed in this PR. Specifically we examined each individual code change to understand why it improved performance with the intention of applying the smallest possible change with the biggest net impact. We are proposing more minimal change that actually has a greater performance impact in #482. We appreciate you drawing our attention to this potential improvement. Thank you for the continued collaboration! Sincerely, |
|
This PR has been superseded by PR #482. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes https://jira.mongodb.org/browse/CSHARP-2450.
Kindly review my individual check-ins as all 3 check-ins affect the same file and will be confusing in a single comparison.
The most recent check-in contains the most important change: This is where I trade a tiny little bit of increasing complexity (cloning a small HashSet) on the way less likely write-scenario for a substantially better performing read-scenario.